feat: adding support for multiqueue#650
Conversation
drew-viles
requested review from
65278,
mcbenjemaa,
pborn-ionos and
wikkyk
as code owners
TestsPlease note that running unit and e2e tests requires manual approval from a team member. e2e testsWe use labels to control which e2e tests contexts are run:
ℹ️ Ask a team member to add the requested labels if you don't have enough permissions. |
|
Nice and simple change but we'll hold it for now until after we have released 0.8.0 and v1alpha2. |
wikkyk
left a comment
wikkyk
left a comment
There was a problem hiding this comment.
Please port this to v1alpha2!
Remember to update the tests with the correct type, too.
|
|
Sorry absolutely slammed atm, I will pick this up again asap though |
|
v0.8.0 has been released so we are reopening feature merges. Please port your PR to v1alpha2 and address my comments and we'll do another review. This is a good feature, I would really like it to go in. |
|
I'll looks at getting it done this weekend. Time has been not been on my side the past few weeks. |
drew-viles
force-pushed
the
network-multiqueue
branch
from
28aa04f to
12f91d3
Compare
|
ok, I think I've got everything covered, |
| }) | ||
|
|
||
| Context("Queues", func() { | ||
| It("Should not allow machine with network device queue equal to 0", func() { |
There was a problem hiding this comment.
Please add a test for >65535, a test for 0<queues<65536, and a test for nil.
| // example 'virtio,bridge=vmbr0,tag=100'. | ||
| func formatNetworkDevice(model, bridge string, mtu *int32, vlan *int32) string { | ||
| // example 'virtio,bridge=vmbr0,tag=100,queues=4'. | ||
| func formatNetworkDevice(model, bridge string, mtu *int32, vlan *int32, queues int32) string { |
There was a problem hiding this comment.
This function takes pointers to optional values, please follow that pattern.
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
wikkyk
force-pushed
the
network-multiqueue
branch
from
aafde41 to
609d7e0
Compare
2 participants
Issue #, if available:
#501
Description of changes:
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
Testing performed:
Added tests for
make testand ran with success. I'm currently trying to get access to a proxmox with shared storage as it appears to be required for this where as right now, I'm using local storage.If you want to wait for that testing to be done on my side I totally get it! However, the change seems pretty straight forward on the whole so I'll let you make the call from your side.